From 030f4f719e72a7bc29119e0d37de90f0d6e01b51 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 15 May 2025 12:28:10 +0200 Subject: [PATCH] fix(delete): optimize item exist checks when propagating from server QFileInfo::exists(filename) is the fastest method alos avoid creating too many QFileInfo instances when we need it for multiple purposes Signed-off-by: Matthieu Gallien --- src/common/filesystembase.cpp | 3 +-- src/libsync/propagatorjobs.cpp | 21 ++++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index d2497a301..41919e59f 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -392,8 +392,7 @@ bool FileSystem::fileExists(const QString &filename, const QFileInfo &fileInfo) // not valid. There needs to be one initialised here. Otherwise the incoming // fileInfo is re-used. if (fileInfo.filePath() != filename) { - QFileInfo myFI(filename); - re = myFI.exists(); + re = QFileInfo::exists(filename); } return re; } diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 072ccbfd7..c2b929d83 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -108,23 +108,26 @@ void PropagateLocalRemove::start() QString removeError; if (_moveToTrash && propagator()->syncOptions()._vfs->mode() != OCC::Vfs::WindowsCfApi) { - if ((QDir(filename).exists() || FileSystem::fileExists(filename)) - && !FileSystem::moveToTrash(filename, &removeError)) { - qCWarning(lcPropagateLocalRemove()) << "move to trash failed" << filename << removeError; - done(SyncFileItem::NormalError, tr("Temporary error when removing local item removed from server."), ErrorCategory::GenericError); - return; + const auto fileInfo = QFileInfo{filename}; + if (FileSystem::fileExists(filename, fileInfo)) { + if (!FileSystem::moveToTrash(filename, &removeError)) { + qCWarning(lcPropagateLocalRemove()) << "move to trash failed" << filename << removeError; + done(SyncFileItem::NormalError, tr("Temporary error when removing local item removed from server."), ErrorCategory::GenericError); + return; + } + } else { + qCWarning(lcPropagateLocalRemove()) << "move to trash failed" << filename << "was already deleted"; } } else { if (_item->isDirectory()) { - if (QDir(filename).exists() && !removeRecursively(QString())) { + if (FileSystem::fileExists(filename) && !removeRecursively(QString())) { done(SyncFileItem::NormalError, tr("Temporary error when removing local item removed from server."), ErrorCategory::GenericError); return; } } else { - if (FileSystem::fileExists(filename)) { - const auto fileInfo = QFileInfo{filename}; + const auto fileInfo = QFileInfo{filename}; + if (FileSystem::fileExists(filename, fileInfo)) { const auto parentFolderPath = fileInfo.dir().absolutePath(); - const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; if (!FileSystem::remove(filename, &removeError)) { -- 2.30.2